Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(duplication): prevent plog files from being removed by GC while they are being checked by duplication #1597

Merged
merged 17 commits into from
Mar 28, 2024
Merged

fix(duplication): prevent plog files from being removed by GC while they are being checked by duplication #1597

merged 17 commits into from
Mar 28, 2024

Conversation

ninsmiracle
Copy link
Contributor

@ninsmiracle ninsmiracle commented Sep 1, 2023 •
edited by empiredan
Loading

What problem does this PR solve?

#1596

What is changed and how does it work?

Using an atomic member to prevent plog files from being removed by GC
when the plog files are being checked by duplication.

Tests
  • Unit test
    -cluster test

@github-actions github-actions bot added the cpp label Sep 1, 2023
@acelyc111
Copy link
Member

@ninsmiracle Thanks for the contribution!

This pull request mention "slog" but I didn't see how does slog been operated in the patch, could you please describe the relationship? Thanks!

@ninsmiracle
Copy link
Contributor Author

ninsmiracle commented Sep 13, 2023 •
edited by acelyc111
Loading

@ninsmiracle Thanks for the contribution!

This pull request mention "slog" but I didn't see how does slog been operated in the patch, could you please describe the relationship? Thanks!

In my opinion, this PR should called "plog gc crash",however in our inside gitlab repository,this commit called this name.I guess it's may be related to old version Slog ? But I'm not familiar with old version pegasus. So could I change the PR's name?

@acelyc111
Copy link
Member

@ninsmiracle Thanks for the contribution!
This pull request mention "slog" but I didn't see how does slog been operated in the patch, could you please describe the relationship? Thanks!

In my opinion, this PR should called "plog gc crash",however in our inside gitlab repository,this commit called this name.I guess it's may be related to old version Slog ? But I'm not familiar with old version pegasus. So could I change the PR's name?

Feel free to rename.

@ninsmiracle ninsmiracle changed the title fix(duplication): fix slog gc crash fix(duplication): fix plog gc crash Sep 19, 2023
acelyc111
acelyc111 previously approved these changes Oct 11, 2023
View reviewed changes
src/replica/replica.h Show resolved Hide resolved
@ninsmiracle ninsmiracle changed the title fix(duplication): fix plog gc crash fix: (duplication) fix plog gc crash Jan 29, 2024
@ninsmiracle ninsmiracle changed the title fix: (duplication) fix plog gc crash fix(duplication): fix plog gc crash Jan 29, 2024
acelyc111
acelyc111 reviewed Mar 7, 2024
View reviewed changes
src/replica/replica.h Outdated Show resolved Hide resolved
acelyc111
acelyc111 previously approved these changes Mar 7, 2024
View reviewed changes
empiredan
empiredan previously approved these changes Mar 27, 2024
View reviewed changes
acelyc111
acelyc111 previously approved these changes Mar 27, 2024
View reviewed changes
@ninsmiracle ninsmiracle dismissed stale reviews from acelyc111 and empiredan via ed0b3c7 March 28, 2024 03:40
@empiredan empiredan changed the title fix(duplication): fix plog gc crash fix(duplication): prevent plog files from being removed by GC while they are being checked by duplication Mar 28, 2024
@empiredan empiredan merged commit 17cb002 into apache:master Mar 28, 2024
57 of 59 checks passed
@empiredan empiredan added type/bug-fix This PR fixes a bug. component/duplication cluster duplication labels Mar 28, 2024
@empiredan empiredan mentioned this pull request Apr 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acelyc111 acelyc111 acelyc111 approved these changes

@empiredan empiredan empiredan approved these changes

Assignees
No one assigned
Labels
component/duplication cluster duplication cpp type/bug-fix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ninsmiracle @acelyc111 @empiredan